Fixes voice call button disappearing in DM rooms with more than 2 members#5548
Merged
ericdecanini merged 6 commits intodevelopfrom Mar 29, 2022
Merged
Fixes voice call button disappearing in DM rooms with more than 2 members#5548ericdecanini merged 6 commits intodevelopfrom
ericdecanini merged 6 commits intodevelopfrom
Conversation
Unit Test Results106 files + 8 106 suites +8 1m 6s ⏱️ -1s Results for commit bfd31de. ± Comparison against base commit e0b93c2. This pull request removes 12 and adds 20 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
ericdecanini
requested review from
a team and
onurays
and removed request for
a team
onurays
reviewed
Mar 18, 2022
| ) | ||
|
|
||
| fun isWebRTCCallOptionAvailable() = (asyncRoomSummary.invoke()?.joinedMembersCount ?: 0) <= 2 | ||
| fun isWebRTCCallOptionAvailable() = asyncRoomSummary.invoke()?.isDirect ?: true |
Contributor
There was a problem hiding this comment.
I don't have enough context about our support for more than 2 people. Web client is creating a Jitsi video call widget even if you click to voice call button. @ganfra what do you think?
Member
There was a problem hiding this comment.
Yes WebRtc is only working with one opponent, otherwise it should be a jitsi call (with video disabled then)
Contributor
Author
There was a problem hiding this comment.
I suppose this must be why it was disabled previously. Thanks for the context, I'll reapproach this!
Edit: Upon looking at the code, the call should still be working as a Jitsi audio call out-the-box. I renamed the above function to be more agnostic towards the type of call
onurays
approved these changes
Mar 29, 2022
Contributor
onurays
left a comment
onurays
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the update.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of change
Content
Changes the condition for the voice call option on the action bar showing from being in a room of 2 or less people to simply being in a direct message room.
Motivation and context
Closes #4762
Quoting conversation in the issue (re current behaviour of voice call not showing):
"this is not a normal behaviour for a DM based group chat (using native VoIP system). We should have both voice and video calls displayed.
For "group rooms" e.g: the large Matrix community room, we currently use Jitsi and so only conference calls should be displayed with a video button. The conference calls are controlled by admins with permissions."
Screenshots / GIFs
Tests
Tested devices
Checklist